Skip to content

DEPR: Remove *args and **kwargs on resample methods #51042

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 9 commits into from
Feb 5, 2023
Merged

DEPR: Remove *args and **kwargs on resample methods #51042

merged 9 commits into from
Feb 5, 2023

Conversation

kostyafarber
Copy link
Contributor

@phofl
Copy link
Member

phofl commented Jan 28, 2023

We want to issue a deprecation warning first before removing them

@kostyafarber
Copy link
Contributor Author

Sorry misunderstood. Do we want something like the warnings that were removed in this #50338?

@phofl
Copy link
Member

phofl commented Jan 28, 2023

Yep the FutureWarning that was removed there

@kostyafarber
Copy link
Contributor Author

Do I have to add tests for the warnings?

@phofl
Copy link
Member

phofl commented Jan 29, 2023

Yep, something with tm.assert_produces_warning

phofl
phofl reviewed Jan 31, 2023
View reviewed changes
Copy link
Member

@phofl phofl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think something went wrong during merging

@kostyafarber
Copy link
Contributor Author

What exactly?

@kostyafarber
Copy link
Contributor Author

What exactly?

@phofl

@phofl
Copy link
Member

phofl commented Feb 2, 2023

Your diff in v2.0.0 is huge, that is odd

@kostyafarber
Copy link
Contributor Author

Hmm it is. I may of accidentally deleted something and added it back. If I'm not getting any merge conflicts on my end then it should be okay I guess.

@phofl
Copy link
Member

phofl commented Feb 2, 2023

No not really, you have to revert the unnecessary changes

@kostyafarber
Copy link
Contributor Author

Okay will look into it.

@kostyafarber
Copy link
Contributor Author

kostyafarber commented Feb 2, 2023 •
edited
Loading

@phofl latest commit looks okay to me (i.e one before the merge)

@phofl
Copy link
Member

phofl commented Feb 2, 2023

diff is still showing 1k lines, e.g. https://github.com/pandas-dev/pandas/pull/51042/files

@mroeschke mroeschke added Resample resample method Deprecate Functionality to remove in pandas labels Feb 3, 2023
@kostyafarber
Copy link
Contributor Author

@phofl done (I hope)

phofl
phofl reviewed Feb 4, 2023
View reviewed changes
Copy link
Member

@phofl phofl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small comments

@kostyafarber kostyafarber requested a review from phofl February 4, 2023 20:49
phofl
phofl reviewed Feb 4, 2023
View reviewed changes
phofl
phofl reviewed Feb 4, 2023
View reviewed changes
Copy link
Member

@phofl phofl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small comment, otherwise ready

@kostyafarber kostyafarber requested a review from phofl February 4, 2023 21:15
@phofl phofl added this to the 2.0 milestone Feb 4, 2023
@phofl phofl merged commit 76025a6 into pandas-dev:main Feb 5, 2023
@phofl
Copy link
Member

phofl commented Feb 5, 2023

thx @kostyafarber

@phofl phofl mentioned this pull request Feb 5, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@phofl phofl phofl approved these changes

Assignees
No one assigned
Labels
Deprecate Functionality to remove in pandas Resample resample method
Projects
None yet
Milestone
2.0
Development

Successfully merging this pull request may close these issues.

DEPR: *args, **kwargs on resample methods
3 participants
@kostyafarber @phofl @mroeschke